feat(drive): add +sync for two-way local/Drive sync#873
Open
fangshuyu-768 wants to merge 1 commit into
Open
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis pull request adds time-based incremental sync features across Drive shortcuts. The changes introduce smart ChangesDrive Smart Time-Based Sync and Bidirectional Sync
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
github-actions
Bot
added
domain/ccm
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #873 +/- ##
==========================================
+ Coverage 65.92% 66.11% +0.19%
==========================================
Files 523 524 +1
Lines 49687 49979 +292
==========================================
+ Hits 32757 33045 +288
- Misses 14131 14136 +5
+ Partials 2799 2798 -1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
🚀 PR Preview Install Guide🧰 CLI updatenpm i -g https://pkg.pr.new/larksuite/cli/@larksuite/cli@72efad502ed6eece6df12f60ac25f3e757f4c9aa🧩 Skill updatenpx skills add larksuite/cli#feat/drive-sync -y -g |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
tests/cli_e2e/drive/drive_status_workflow_test.go (1)
412-426: 💤 Low valueConsider importing epoch parsing logic from production code.
The
mustParseDriveEpochForE2Ehelper duplicates the resolution-inference logic that exists in the production codebase (layer 1: epoch parsing helpers inshortcuts/drive/list_remote.go). While test-specific helpers are valuable for isolation, if the production parsing function is exported, importing it would reduce duplication and ensure consistency.However, this duplication is acceptable for E2E test clarity and independence, so this is purely an optional refactor.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/cli_e2e/drive/drive_status_workflow_test.go` around lines 412 - 426, The helper mustParseDriveEpochForE2E duplicates epoch resolution logic; replace its logic by calling the production parsing helper (the exported epoch parse function used in shortcuts/drive/list_remote.go) instead of reimplementing it: update mustParseDriveEpochForE2E to invoke that exported function (or import and use it directly in the test) so tests use the canonical parsing implementation and avoid duplication while preserving the same error handling behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@shortcuts/drive/drive_pull.go`:
- Around line 332-343: In drivePullApplyRemoteModifiedTime, treat os.Chtimes
failures as best-effort: instead of returning an error after os.Chtimes fails,
log a warning containing the error and continue (return nil) so a successful
content download is not marked failed; update the os.Chtimes error branch in
drivePullApplyRemoteModifiedTime (which follows ResolvePath and uses os.Chtimes)
to emit a warning (e.g., output.Warnf or equivalent logger) mentioning that
preserving remote modified_time failed and include err, then return nil.
In `@shortcuts/drive/drive_sync.go`:
- Around line 323-345: The current conflict-handling branch renames the local
file (os.Rename using oldAbsPath -> newAbsPath with suffixedRel) before calling
drivePullDownload, risking loss of the original path if the download fails;
change the flow to either (A) download the remote file to a temporary path
(e.g., under the same directory) and only rename/move it into entry.RelPath
after drivePullDownload succeeds, or (B) if you must rename first, perform the
rename as you do now (oldAbsPath -> newAbsPath) but on drivePullDownload failure
rollback by renaming newAbsPath back to oldAbsPath and record the failure;
update items (driveSyncItem) consistently in both success and rollback paths and
ensure you reference pullRemoteFiles[entry.RelPath], drivePullDownload,
oldAbsPath/newAbsPath, and suffixedRel when making the change.
- Around line 395-408: driveSyncAskConflict currently ignores Fscanln errors and
treats empty answers as "remote-wins"; change the input handling to capture and
check the error from fmt.Fscanln in driveSyncAskConflict, and on any read
failure (err != nil) print a brief diagnostic to runtime.IO().ErrOut and return
the skip outcome (empty string) or propagate an error instead of falling through
to the default remote-wins branch; only proceed to TrimSpace/ToLower and the
switch when Fscanln succeeded so that the default case remains remote-wins for
valid empty input but not for unreadable stdin. Ensure you modify the call site
using fmt.Fscanln to capture the error and reference driveSyncAskConflict,
runtime.IO(), and the existing return values
driveSyncOnConflictRemoteWins/KeepBoth/LocalWins to locate the code.
In `@skills/lark-drive/SKILL.md`:
- Line 232: The inline code token `detection=exact|quick` inside the table cell
is breaking the Markdown table because the pipe is parsed as a column separator;
update that token to escape the pipe (e.g., change to `detection=exact\|quick`)
or wrap the whole cell content in a code span that preserves the literal pipe so
the table renders correctly; locate the occurrence of the literal string
detection=exact|quick in SKILL.md and apply the escape so the table columns
remain intact.
---
Nitpick comments:
In `@tests/cli_e2e/drive/drive_status_workflow_test.go`:
- Around line 412-426: The helper mustParseDriveEpochForE2E duplicates epoch
resolution logic; replace its logic by calling the production parsing helper
(the exported epoch parse function used in shortcuts/drive/list_remote.go)
instead of reimplementing it: update mustParseDriveEpochForE2E to invoke that
exported function (or import and use it directly in the test) so tests use the
canonical parsing implementation and avoid duplication while preserving the same
error handling behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 1290dfb2-d131-4917-8e32-de7866021c71
📒 Files selected for processing (20)
shortcuts/drive/drive_duplicate_remote_test.goshortcuts/drive/drive_pull.goshortcuts/drive/drive_pull_test.goshortcuts/drive/drive_push.goshortcuts/drive/drive_push_test.goshortcuts/drive/drive_status.goshortcuts/drive/drive_status_test.goshortcuts/drive/drive_sync.goshortcuts/drive/drive_sync_test.goshortcuts/drive/list_remote.goshortcuts/drive/shortcuts.goshortcuts/drive/shortcuts_test.goskills/lark-drive/SKILL.mdskills/lark-drive/references/lark-drive-pull.mdskills/lark-drive/references/lark-drive-push.mdskills/lark-drive/references/lark-drive-status.mdtests/cli_e2e/drive/drive_pull_dryrun_test.gotests/cli_e2e/drive/drive_push_dryrun_test.gotests/cli_e2e/drive/drive_status_dryrun_test.gotests/cli_e2e/drive/drive_status_workflow_test.go
fangshuyu-768
changed the title
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
skills/lark-drive/SKILL.md (1)
232-234:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winEscape
|inside inline code to keep the shortcuts table valid.Line 232 and Line 234 contain unescaped pipes inside table cells, which breaks column parsing in Markdown renderers.
Suggested fix
-| [`+status`](references/lark-drive-status.md) | Compare a local directory with a Drive folder by exact SHA-256 hash by default, or use `--quick` for a best-effort modified-time diff that skips remote downloads; reports `new_local` / `new_remote` / `modified` / `unchanged` plus `detection=exact|quick`. Duplicate remote `rel_path` conflicts fail fast with `error.type=duplicate_remote_path` and list every conflicting entry; do not proceed as if one was chosen. `--local-dir` 必须是 cwd 内的相对路径，越界路径 CLI 会直接拒绝；目标在 cwd 外时引导用户切换 agent 工作目录，不要私自 `cd` 绕过。 | +| [`+status`](references/lark-drive-status.md) | Compare a local directory with a Drive folder by exact SHA-256 hash by default, or use `--quick` for a best-effort modified-time diff that skips remote downloads; reports `new_local` / `new_remote` / `modified` / `unchanged` plus `detection=exact\|quick`. Duplicate remote `rel_path` conflicts fail fast with `error.type=duplicate_remote_path` and list every conflicting entry; do not proceed as if one was chosen. `--local-dir` 必须是 cwd 内的相对路径，越界路径 CLI 会直接拒绝；目标在 cwd 外时引导用户切换 agent 工作目录，不要私自 `cd` 绕过。 | -| `+sync` | Two-way local ↔ Drive sync. Reuses `+status` diff buckets, pulls `new_remote`, pushes `new_local`, and resolves `modified` via `--on-conflict=remote-wins|local-wins|keep-both|ask`. `--quick` enables modified-time diffing, `--on-duplicate-remote` supports `fail|newest|oldest`, and the command is intentionally non-destructive (no delete on either side). | +| `+sync` | Two-way local ↔ Drive sync. Reuses `+status` diff buckets, pulls `new_remote`, pushes `new_local`, and resolves `modified` via `--on-conflict=remote-wins\|local-wins\|keep-both\|ask`. `--quick` enables modified-time diffing, `--on-duplicate-remote` supports `fail\|newest\|oldest`, and the command is intentionally non-destructive (no delete on either side). |🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@skills/lark-drive/SKILL.md` around lines 232 - 234, Table cells in SKILL.md contain unescaped pipe characters inside inline code which breaks Markdown table parsing; locate the inline code tokens in the table for `+status` (`detection=exact|quick`), `+sync` (`--on-conflict=remote-wins|local-wins|keep-both|ask`) and `+sync`/`+pull` spots with option enumerations (e.g. `--on-duplicate-remote` values `fail|newest|oldest`) and escape every `|` as `\|` (or alternatively wrap the entire cell in a code span/block) so the pipes are treated literally and the table columns render correctly.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@shortcuts/drive/drive_sync_test.go`:
- Around line 612-614: The test currently slices out using strings.Index without
checking the result which can panic if `"items"` is missing; change the logic in
the test around the itemsSection creation (the code using strings.Index(out,
`"items"`) and variable itemsSection) to first capture the index,
assert/index-check it is >= 0 (e.g. t.Fatalf/t.Errorf including the full output
if missing), and only then slice out[idx:] and run the existing containment
assertion for `"rel_path": "a.txt"`.
---
Duplicate comments:
In `@skills/lark-drive/SKILL.md`:
- Around line 232-234: Table cells in SKILL.md contain unescaped pipe characters
inside inline code which breaks Markdown table parsing; locate the inline code
tokens in the table for `+status` (`detection=exact|quick`), `+sync`
(`--on-conflict=remote-wins|local-wins|keep-both|ask`) and `+sync`/`+pull` spots
with option enumerations (e.g. `--on-duplicate-remote` values
`fail|newest|oldest`) and escape every `|` as `\|` (or alternatively wrap the
entire cell in a code span/block) so the pipes are treated literally and the
table columns render correctly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: fa906003-d31e-4652-b217-a8e90b8bd977
📒 Files selected for processing (3)
shortcuts/drive/drive_sync.goshortcuts/drive/drive_sync_test.goskills/lark-drive/SKILL.md
🚧 Files skipped from review as they are similar to previous changes (1)
- shortcuts/drive/drive_sync.go
Comment on lines
+612
to
+614
| itemsSection := out[strings.Index(out, `"items"`):] | ||
| if strings.Contains(itemsSection, `"rel_path": "a.txt"`) { | ||
| t.Errorf("a.txt should not appear in items[] (mtime matches remote, should be unchanged)\noutput: %s", out) |
There was a problem hiding this comment.
Guard the items section lookup before slicing.
Line 612 can panic when "items" is missing, which makes failures less diagnosable than a normal test assertion.
Suggested fix
- itemsSection := out[strings.Index(out, `"items"`):]
+ idx := strings.Index(out, `"items"`)
+ if idx == -1 {
+ t.Fatalf(`expected "items" section in output, got: %s`, out)
+ }
+ itemsSection := out[idx:]
if strings.Contains(itemsSection, `"rel_path": "a.txt"`) {
t.Errorf("a.txt should not appear in items[] (mtime matches remote, should be unchanged)\noutput: %s", out)
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| itemsSection := out[strings.Index(out, `"items"`):] | |
| if strings.Contains(itemsSection, `"rel_path": "a.txt"`) { | |
| t.Errorf("a.txt should not appear in items[] (mtime matches remote, should be unchanged)\noutput: %s", out) | |
| idx := strings.Index(out, `"items"`) | |
| if idx == -1 { | |
| t.Fatalf(`expected "items" section in output, got: %s`, out) | |
| } | |
| itemsSection := out[idx:] | |
| if strings.Contains(itemsSection, `"rel_path": "a.txt"`) { | |
| t.Errorf("a.txt should not appear in items[] (mtime matches remote, should be unchanged)\noutput: %s", out) | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@shortcuts/drive/drive_sync_test.go` around lines 612 - 614, The test
currently slices out using strings.Index without checking the result which can
panic if `"items"` is missing; change the logic in the test around the
itemsSection creation (the code using strings.Index(out, `"items"`) and variable
itemsSection) to first capture the index, assert/index-check it is >= 0 (e.g.
t.Fatalf/t.Errorf including the full output if missing), and only then slice
out[idx:] and run the existing containment assertion for `"rel_path": "a.txt"`.
There was a problem hiding this comment.
♻️ Duplicate comments (2)
skills/lark-drive/SKILL.md (1)
232-232:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winEscape
|in inline code inside the shortcuts table.Line 232 and Line 234 use unescaped pipes in inline code, which breaks table column parsing (MD056). Escape each
|as\|.Suggested fix
-| [`+status`](references/lark-drive-status.md) | Compare a local directory with a Drive folder by exact SHA-256 hash by default, or use `--quick` for a best-effort modified-time diff that skips remote downloads; reports `new_local` / `new_remote` / `modified` / `unchanged` plus `detection=exact|quick`. Duplicate remote `rel_path` conflicts fail fast with `error.type=duplicate_remote_path` and list every conflicting entry; do not proceed as if one was chosen. `--local-dir` 必须是 cwd 内的相对路径，越界路径 CLI 会直接拒绝；目标在 cwd 外时引导用户切换 agent 工作目录，不要私自 `cd` 绕过。 | +| [`+status`](references/lark-drive-status.md) | Compare a local directory with a Drive folder by exact SHA-256 hash by default, or use `--quick` for a best-effort modified-time diff that skips remote downloads; reports `new_local` / `new_remote` / `modified` / `unchanged` plus `detection=exact\|quick`. Duplicate remote `rel_path` conflicts fail fast with `error.type=duplicate_remote_path` and list every conflicting entry; do not proceed as if one was chosen. `--local-dir` 必须是 cwd 内的相对路径，越界路径 CLI 会直接拒绝；目标在 cwd 外时引导用户切换 agent 工作目录，不要私自 `cd` 绕过。 | -| `+sync` | Two-way local ↔ Drive sync. Reuses `+status` diff buckets, pulls `new_remote`, pushes `new_local`, and resolves `modified` via `--on-conflict=remote-wins|local-wins|keep-both|ask`. `--quick` enables best-effort modified-time diffing (timestamp mismatches can still trigger real pull/push actions), `--on-duplicate-remote` supports `fail|newest|oldest`, and the command is intentionally non-destructive (no delete on either side). | +| `+sync` | Two-way local ↔ Drive sync. Reuses `+status` diff buckets, pulls `new_remote`, pushes `new_local`, and resolves `modified` via `--on-conflict=remote-wins\|local-wins\|keep-both\|ask`. `--quick` enables best-effort modified-time diffing (timestamp mismatches can still trigger real pull/push actions), `--on-duplicate-remote` supports `fail\|newest\|oldest`, and the command is intentionally non-destructive (no delete on either side). |Also applies to: 234-234
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@skills/lark-drive/SKILL.md` at line 232, The table row containing the `+status` shortcut has inline code spans that include pipe characters which break Markdown table parsing (e.g., `detection=exact|quick`, `error.type=duplicate_remote_path`, `new_local`/`new_remote`/`modified`/`unchanged` and flags like `--quick`/`--local-dir`); update the SKILL.md row text so every literal pipe within inline code is escaped as `\|` (for example change `detection=exact|quick` to `detection=exact\|quick`) throughout the `+status` row and any nearby rows (including the similar occurrence around line 234) so the Markdown table renders properly.shortcuts/drive/drive_sync_test.go (1)
752-755:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winGuard the
itemssection lookup before slicing.Line 752 can produce confusing test results when
"items"is missing from the output. Ifstrings.Indexreturns -1,out[-1:]slices from the end, causing the subsequentstrings.Containscheck to search the entire string instead of just the items section, which makes test failures less diagnosable.Suggested fix
- itemsSection := out[strings.Index(out, `"items"`):] + idx := strings.Index(out, `"items"`) + if idx == -1 { + t.Fatalf(`expected "items" section in output, got: %s`, out) + } + itemsSection := out[idx:] if strings.Contains(itemsSection, `"rel_path": "a.txt"`) { t.Errorf("a.txt should not appear in items[] (mtime matches remote, should be unchanged)\noutput: %s", out) }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@shortcuts/drive/drive_sync_test.go` around lines 752 - 755, The test currently slices out using strings.Index(out, `"items"`) without checking for -1 which can produce an unintended slice when `"items"` is absent; fix by checking the index first (e.g., idx := strings.Index(out, `"items"`)) and fail the test or return immediately with a clear error if idx == -1, otherwise compute itemsSection := out[idx:] and then run the existing strings.Contains check for `"rel_path": "a.txt"`; reference the existing symbols itemsSection, out, strings.Index and strings.Contains when applying this guard.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Duplicate comments:
In `@shortcuts/drive/drive_sync_test.go`:
- Around line 752-755: The test currently slices out using strings.Index(out,
`"items"`) without checking for -1 which can produce an unintended slice when
`"items"` is absent; fix by checking the index first (e.g., idx :=
strings.Index(out, `"items"`)) and fail the test or return immediately with a
clear error if idx == -1, otherwise compute itemsSection := out[idx:] and then
run the existing strings.Contains check for `"rel_path": "a.txt"`; reference the
existing symbols itemsSection, out, strings.Index and strings.Contains when
applying this guard.
In `@skills/lark-drive/SKILL.md`:
- Line 232: The table row containing the `+status` shortcut has inline code
spans that include pipe characters which break Markdown table parsing (e.g.,
`detection=exact|quick`, `error.type=duplicate_remote_path`,
`new_local`/`new_remote`/`modified`/`unchanged` and flags like
`--quick`/`--local-dir`); update the SKILL.md row text so every literal pipe
within inline code is escaped as `\|` (for example change
`detection=exact|quick` to `detection=exact\|quick`) throughout the `+status`
row and any nearby rows (including the similar occurrence around line 234) so
the Markdown table renders properly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 3abea75e-7c76-4004-acd0-2069aeec3e5c
📒 Files selected for processing (5)
shortcuts/drive/drive_push.goshortcuts/drive/drive_push_test.goshortcuts/drive/drive_sync.goshortcuts/drive/drive_sync_test.goskills/lark-drive/SKILL.md
🚧 Files skipped from review as they are similar to previous changes (3)
- shortcuts/drive/drive_push.go
- shortcuts/drive/drive_sync.go
- shortcuts/drive/drive_push_test.go
fangshuyu-768
force-pushed
the
feat/drive-sync
branch
from
be2ac35 to
ee47bb8
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@skills/lark-drive/SKILL.md`:
- Line 234: The table cell for `+sync` contains unescaped pipe characters in
option examples (e.g., `--on-conflict=...` and
`--on-duplicate-remote=fail|newest|oldest`) which breaks Markdown table parsing;
update the `+sync` row so any `|` inside inline code is escaped (e.g., `\|` or
HTML entity `|`) or wrap the entire option list in a protected code span so
`--on-conflict`, `--on-duplicate-remote` and the `fail|newest|oldest` example
render as a single table cell without splitting columns.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: e0816faf-fad9-44bb-9c24-6b170d45aae9
📒 Files selected for processing (20)
shortcuts/drive/drive_duplicate_remote_test.goshortcuts/drive/drive_pull.goshortcuts/drive/drive_pull_test.goshortcuts/drive/drive_push.goshortcuts/drive/drive_push_test.goshortcuts/drive/drive_status.goshortcuts/drive/drive_status_test.goshortcuts/drive/drive_sync.goshortcuts/drive/drive_sync_test.goshortcuts/drive/list_remote.goshortcuts/drive/shortcuts.goshortcuts/drive/shortcuts_test.goskills/lark-drive/SKILL.mdskills/lark-drive/references/lark-drive-pull.mdskills/lark-drive/references/lark-drive-push.mdskills/lark-drive/references/lark-drive-status.mdtests/cli_e2e/drive/drive_pull_dryrun_test.gotests/cli_e2e/drive/drive_push_dryrun_test.gotests/cli_e2e/drive/drive_status_dryrun_test.gotests/cli_e2e/drive/drive_status_workflow_test.go
✅ Files skipped from review due to trivial changes (3)
- shortcuts/drive/shortcuts_test.go
- skills/lark-drive/references/lark-drive-pull.md
- skills/lark-drive/references/lark-drive-status.md
🚧 Files skipped from review as they are similar to previous changes (13)
- shortcuts/drive/shortcuts.go
- shortcuts/drive/list_remote.go
- shortcuts/drive/drive_duplicate_remote_test.go
- tests/cli_e2e/drive/drive_push_dryrun_test.go
- tests/cli_e2e/drive/drive_status_dryrun_test.go
- skills/lark-drive/references/lark-drive-push.md
- shortcuts/drive/drive_sync.go
- shortcuts/drive/drive_status.go
- shortcuts/drive/drive_status_test.go
- tests/cli_e2e/drive/drive_status_workflow_test.go
- shortcuts/drive/drive_push.go
- shortcuts/drive/drive_pull.go
- shortcuts/drive/drive_sync_test.go
fangshuyu-768
force-pushed
the
feat/drive-sync
branch
from
ee47bb8 to
2ae9e3a
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@skills/lark-drive/references/lark-drive-push.md`:
- Line 70: The doc contains duplicated entries for the `--if-exists` option (the
table row, bullet list entries, and the "smart" mode paragraph are repeated);
remove the duplicate occurrences so there is only one canonical description of
`--if-exists` and its enum values (`skip`, `smart`, `overwrite`) and keep a
single explanatory paragraph for the "smart" mode — update the sections that
reference `--if-exists` (including the repeated table row and the repeated
smart-mode paragraph) to eliminate redundancy while preserving the single
authoritative wording.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: d82c44a8-4dfd-429d-882c-d6ba3e9b7bdc
📒 Files selected for processing (14)
shortcuts/drive/drive_pull_test.goshortcuts/drive/drive_push.goshortcuts/drive/drive_push_test.goshortcuts/drive/drive_status.goshortcuts/drive/drive_status_test.goshortcuts/drive/drive_sync.goshortcuts/drive/drive_sync_test.goshortcuts/drive/shortcuts.goshortcuts/drive/shortcuts_test.goskills/lark-drive/SKILL.mdskills/lark-drive/references/lark-drive-push.mdskills/lark-drive/references/lark-drive-status.mdtests/cli_e2e/drive/drive_status_dryrun_test.gotests/cli_e2e/drive/drive_status_workflow_test.go
💤 Files with no reviewable changes (1)
- shortcuts/drive/drive_pull_test.go
✅ Files skipped from review due to trivial changes (1)
- skills/lark-drive/references/lark-drive-status.md
🚧 Files skipped from review as they are similar to previous changes (8)
- shortcuts/drive/shortcuts_test.go
- shortcuts/drive/shortcuts.go
- tests/cli_e2e/drive/drive_status_dryrun_test.go
- shortcuts/drive/drive_sync.go
- shortcuts/drive/drive_status_test.go
- shortcuts/drive/drive_status.go
- tests/cli_e2e/drive/drive_status_workflow_test.go
- shortcuts/drive/drive_sync_test.go
fangshuyu-768
force-pushed
the
feat/drive-sync
branch
2 times, most recently
from
f4c18cd to
40850c7
Compare
Add a dedicated drive +sync shortcut so repeated local/remote directory syncs can be handled in one workflow with explicit conflict policies and stronger coverage for shared diff logic.
fangshuyu-768
force-pushed
the
feat/drive-sync
branch
from
40850c7 to
72efad5
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR adds
drive +syncfor two-way sync between a local directory and a Drive folder.What’s included
drive +syncto compute a diff and apply sync actions in one command+status,+pull, and+pushflow for file discovery and transferremote-wins(default)local-winskeep-bothask--quicksupport so+status/+synccan use modified time comparison instead of SHA-256--if-exists=smarttodrive +pullanddrive +pushso repeated syncs can skip files that are already up to date based on modified time+syncnon-destructive: it only adds or updates files and does not delete files on either sideOutput
+syncreturns:exactorquick)new_local,new_remote,modified,unchanged)Test Plan
--if-exists=smartin+pulland+push+pull,+push, and+status+status --quickgo build ./...Summary by CodeRabbit
New Features
drive +synccommand for bidirectional synchronization between a local directory and Drive folder, with configurable conflict resolution options--quickflag todrive +statusfor rapid detection using modification times instead of SHA-256 hashing--if-exists=smartsupport todrive +pushand enhanceddrive +pullwith smart policy for timestamp-based efficient skippingDocumentation